fix: repair guest boot chain and harden host-side sandbox - #17
Conversation
The guest boot chain had four independent blockers that each stopped execution before user code could run: - Boot args passed init=/usr/local/bin/ignite-guest-agent while create_rootfs installed the agent at /sbin/init. Both now use a shared GUEST_INIT_PATH constant. - The agent was built without --target x86_64-unknown-linux-musl, producing a glibc-linked binary for a rootfs with no libc or dynamic loader. It now targets musl, and both build_guest_agent and create_rootfs reject an ELF carrying a PT_INTERP segment. - The agent called mkdir on a read-only rootfs. Mount points are now baked into the image, devtmpfs is mounted first (without it /dev/vdb does not exist), and a failed disk mount is fatal rather than logged and ignored. - download_kernel copied /boot/vmlinuz-*, a compressed bzImage that Firecracker cannot boot. It now requires an uncompressed ELF vmlinux and names any compressed images it found. The VSOCK listener also binds before InstanceStart, so the guest cannot exhaust its connect retries before the host is listening. Security fixes: - service.name from service.yaml reached a host path that boot() unlinks, allowing arbitrary file removal via `../..`. It is now validated and sanitized, and the socket path is pinned under /tmp. - The timeout watchdog only checked its deadline in the WouldBlock branch, so a guest writing output continuously ran forever. The deadline is now checked every iteration. - Guest-controlled frame lengths were allocated verbatim (up to 4 GiB). Frames are capped at 4 MiB and retained output at 8 MiB per stream. - The rate limiter keyed on the spoofable X-Forwarded-For header, never evicted buckets, and compared API keys non-constant-time. It now keys on the transport peer, bounds bucket count, and compares in constant time. - CORS defaulted to any origin on an endpoint that executes code. It is now opt-in via IGNITE_CORS_ORIGINS, and both entry points warn when IGNITE_API_KEY is unset. - The agent clears the environment before applying host-supplied vars. Flags that parsed but did nothing are wired up (--runtime, --console-out) or now fail loudly (--audit, which would otherwise report a clean audit for code that was never audited). cpuLimit rounds up instead of truncating 0.5 to 0 vCPUs; runtime versions resolve to runtimes/<name>@<version> with a logged fallback; cold-start timing is reported only when the guest emits it rather than fabricated. Also fixes send_put_uds treating a 4xx whose body echoed "HTTP/1.1 200" as success, `serve --host localhost` silently falling back to 127.0.0.1 instead of resolving, and the KVM check in `status` misreading mode bits so a healthy device reported "check user group". Rewrites AGENTS.md, which documented a Bun/TypeScript monorepo that no longer exists, and corrects README/docs claims about macOS support, audit mode, and runtime provisioning. Adds a "Not Implemented" section to the threat model covering audit mode, the Firecracker jailer, macOS, and unauthenticated-by-default HTTP. Test coverage goes from 1 test to 29. Verification: cargo fmt --check, clippy -D warnings, and cargo test --workspace all pass. The musl agent was confirmed static-pie with no PT_INTERP and byte-identical to /sbin/init extracted from the built rootfs image. VM-level execution could NOT be verified: this host has no firecracker binary and no uncompressed vmlinux, so the guest-side path (devtmpfs mount, VSOCK handshake, PID 1 startup) is fixed by inspection only and still needs a real boot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guest agent runs as PID 1 on a rootfs with no libc and no dynamic loader. If it regresses to dynamic linking the guest cannot boot at all, and nothing in the host-side test suite catches it — the failure only appears when a real VM fails to exec init. Build it for x86_64-unknown-linux-musl in CI and fail the job if the resulting ELF carries a PT_INTERP segment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request hardens Linux/Firecracker execution. It adds static guest-agent and kernel validation, bounded VSOCK I/O, runtime overrides, HTTP authentication and rate limits, incomplete setup reporting, and documentation for current platform support and security limits. ChangesRuntime hardening and execution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant IgniteCore
participant Firecracker
participant GuestAgent
participant Service
CLI->>IgniteCore: submit runtime and execution options
IgniteCore->>Firecracker: configure VM and bind VSOCK listener
Firecracker->>GuestAgent: boot guest and establish session
GuestAgent->>Service: mount runtime and service disks
GuestAgent-->>IgniteCore: stream bounded output and execution status
IgniteCore-->>CLI: return result and metrics
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
ignite-core/src/execution.rs (2)
301-390: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the two rejection branches in
execute_service.The new tests cover
resolve_vcpu_count,resolve_runtime_dir, and the stderr parsers. They do not cover the two security-relevant rejections that this change adds:
- invalid
config.service.namerejected at Lines 110-120,options.auditrefused at Lines 125-132.Both branches run before any host dependency such as
mke2fs, a kernel image, or a rootfs. A test that writes a temporary service directory with a craftedservice.yamlreaches them without a hypervisor. Add one blocked case per branch, plus one allowed case that passes name validation and then fails later for a missing runtime.As per coding guidelines: "When security logic changes, include tests proving both allowed and blocked behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ignite-core/src/execution.rs` around lines 301 - 390, Add tests for execute_service covering both security rejection branches and the allowed path: create temporary service directories with crafted service.yaml files, assert invalid config.service.name is rejected, assert options.audit is refused, and verify a valid name/audit configuration passes validation before failing later due to a missing runtime. Keep these tests independent of mke2fs, kernel, rootfs, or hypervisor dependencies.Source: Coding guidelines
159-186: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider validating the kernel and rootfs before building the ext4 images.
create_ext4_imagerunsmke2fsfor the service directory at Line 165 and for the runtime directory at Line 186. The kernel existence check,validate_kernel_image, and the rootfs existence check run afterwards at Lines 198-219. On a host without a validvmlinux, the command performs both image builds and then fails. Moving the cheap path and format checks above the image builds makes the failure immediate.This is ordering only; no behavior changes for a correctly provisioned host.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ignite-core/src/execution.rs` around lines 159 - 186, Move the kernel and rootfs validation flow, including validate_kernel_image and the associated existence/format checks, before the create_ext4_image calls for the service and runtime disks in the execution setup. Preserve all existing validation behavior and image-building behavior for correctly provisioned hosts, but ensure invalid kernel or rootfs paths fail before either image is generated.ignite-core/src/platform/apple_vz.rs (1)
53-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for the unsupported AppleVzOrchestrator behavior.
bootandwait_and_teardownnow returnIgniteError::Runtimewith a stable message. Add anignite-coretest that instantiatesAppleVzOrchestrator, calls both methods, and checks the error variant and stable text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ignite-core/src/platform/apple_vz.rs` around lines 53 - 62, Add an ignite-core regression test for AppleVzOrchestrator that invokes boot and wait_and_teardown, asserting each returns IgniteError::Runtime with the stable unsupported-operation message. Use the existing test conventions and required callback/setup values, and verify both methods independently.Source: Learnings
ignite-core/src/platform/firecracker.rs (1)
355-363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the guest frame-length cap.
The new tests cover the status line, the socket path, the port suffix, and output capping. They do not cover the frame-length cap, which is the check that protects the host against a guest-controlled allocation. The coding guidelines require tests that prove both allowed and blocked behavior when security logic changes.
Extract the bound into a small helper and test it directly, for example
fn frame_length_is_allowed(length: usize) -> bool. Then assert thatMAX_FRAME_BYTESpasses andMAX_FRAME_BYTES + 1is rejected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ignite-core/src/platform/firecracker.rs` around lines 355 - 363, Extract the frame-size comparison from the VSOCK handling path into a small helper such as frame_length_is_allowed(length: usize) -> bool, and use it in the existing MAX_FRAME_BYTES validation. Add direct tests covering both boundary cases: MAX_FRAME_BYTES is allowed, while MAX_FRAME_BYTES + 1 is rejected.Source: Coding guidelines
ignite-guest-agent/src/main.rs (1)
136-165: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider hardening the mount flags.
The
flagsparameter is now caller-controlled, so the safe flags are cheap to add./appcarries untrusted service code, and/dev,/proc, and/sysdo not need setuid or device nodes from the images. AddMS_NOSUID | MS_NODEVto/app,MS_NOSUIDto/runtime(it must stay executable), andMS_NOSUID | MS_NOEXEC | MS_NODEVto/procand/sys.🔒️ Proposed flag hardening
- if let Err(e) = mount_device("proc", "/proc", "proc", 0) { + let pseudo_flags = libc::MS_NOSUID | libc::MS_NOEXEC | libc::MS_NODEV; + if let Err(e) = mount_device("proc", "/proc", "proc", pseudo_flags) { // Not fatal: most runtimes work without /proc, but warn loudly. log_error(&format!("Failed to mount proc at /proc: {}", e)); } - if let Err(e) = mount_device("sysfs", "/sys", "sysfs", 0) { + if let Err(e) = mount_device("sysfs", "/sys", "sysfs", pseudo_flags) { log_error(&format!("Failed to mount sysfs at /sys: {}", e)); } @@ - if let Err(e) = mount_device("/dev/vdb", "/app", "ext4", libc::MS_RDONLY) { + if let Err(e) = mount_device( + "/dev/vdb", + "/app", + "ext4", + libc::MS_RDONLY | libc::MS_NOSUID | libc::MS_NODEV, + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ignite-guest-agent/src/main.rs` around lines 136 - 165, Harden the mount flags at each call site: update the /devtmpfs mount to include MS_NOSUID | MS_NOEXEC | MS_NODEV, add MS_NOSUID | MS_NOEXEC | MS_NODEV to the proc and sysfs mounts, add MS_NOSUID | MS_NODEV while preserving existing flags for the /app mount, and add MS_NOSUID while preserving MS_RDONLY for /runtime. Keep the existing fatal and non-fatal error handling unchanged.ignite-core/src/setup.rs (1)
49-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
validate_kernel_imageandvalidate_agent_is_static.Add unit tests for allowed and blocked inputs: ELF without
PT_INTERPshould pass, ELF withPT_INTERPshould fail, and non-ELF files such as gzip-magic bzImages should fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ignite-core/src/setup.rs` around lines 49 - 93, Add unit tests covering validate_kernel_image and validate_agent_is_static: verify valid ELF data without PT_INTERP passes, ELF data containing PT_INTERP is rejected as dynamically linked, and gzip-magic/non-ELF kernel input is rejected. Build temporary fixture files or in-memory test inputs compatible with read_head, and assert both success and the expected error results.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 49-59: Update the “Verify guest agent links statically” workflow
step to enable pipeline failure propagation and capture the `readelf -l
"$AGENT"` output before checking it. Make the step fail when `readelf` cannot
inspect the binary, and detect dynamic linking by matching the stable `INTERP`
segment tag rather than descriptive wording.
In `@docs/threat-model.md`:
- Line 15: Update the “Bound host memory from guest output” entry in the threat
model to state that the host validates each guest-declared VSOCK frame length
against the 4 MiB limit before allocation, and separately caps retained stdout
and stderr at 8 MiB per stream. Remove the undefined “allocated on the guest's
word” phrasing.
In `@ignite-core/src/platform/firecracker.rs`:
- Around line 511-524: Update cleanup_vm_processes so it removes
self.api_socket_path only when the instance has been configured with an owned
run-specific API socket path; preserve cleanup of the child process and other
instance-owned socket paths. Ensure dropping an unconfigured orchestrator never
unlinks the shared DEFAULT_API_SOCKET.
In `@ignite-core/src/setup.rs`:
- Around line 29-44: Update elf_needs_dynamic_loader to use checked_mul and
checked_add when calculating each program-header offset and the end of the
PT_INTERP field, skipping entries whose arithmetic overflows instead of
panicking or wrapping. Also distinguish static, dynamic, and unknown ELF
classifications so unsupported formats and headers outside ELF_HEADER_SCAN_BYTES
become unknown, then update validate_agent_is_static to reject unknown results.
In `@ignite-http/src/main.rs`:
- Around line 26-32: Reject empty or whitespace-only IGNITE_API_KEY values
during shared configuration parsing, while preserving the warning only for an
unset key; update ignite-http/src/main.rs lines 26-32 and ignite-cli/src/main.rs
lines 1031-1037 so both startup paths return a configuration error instead of
accepting blank credentials, and update docs/api.md lines 73-76 to state that
configured API keys must be nonempty.
In `@ignite-http/src/server.rs`:
- Around line 290-346: Add router-level tests alongside the existing tests that
verify authentication and CORS decisions: accept requests with a valid API key
and configured origin, and reject requests with missing or invalid keys and
origins not present in the allowed configuration. Reuse the router construction
and security configuration symbols already defined in the module, covering both
allowed and blocked outcomes without changing the rate-limiter tests.
- Around line 57-63: Replace the full-map retain block in the rate-limit request
path with per-client timestamp pruning and an expiry index for idle buckets.
Prune only the current client’s timestamps while holding the mutex, and process
expired buckets through the index rather than scanning all entries; preserve the
existing window-based expiration and bucket-cap behavior.
- Around line 271-279: Update the IGNITE_CORS_ORIGINS configuration parsing
before router construction to parse each origin once, reject malformed and
wildcard values, and return a configuration error for any invalid entry or when
no valid origins remain. Store and reuse the validated HeaderValue list in the
CORS setup around AllowOrigin::list, removing the current filter_map behavior
that silently drops invalid origins.
In `@install.sh`:
- Line 26: Update the installer banner’s echo text to describe the actual
enforced microVM mechanism without using the blanket “secure” claim; leave the
surrounding banner formatting and behavior unchanged.
In `@README.md`:
- Line 34: Update the “Host-Reliant Disk Mounts” description in README.md to
state that runtime binaries are installed or downloaded on the host and then
packaged into read-only virtual block devices, while retaining that the guest
agent is built by the setup flow. Remove the claim that Bun, Node, Deno, and
QuickJS are compiled on the host.
---
Nitpick comments:
In `@ignite-core/src/execution.rs`:
- Around line 301-390: Add tests for execute_service covering both security
rejection branches and the allowed path: create temporary service directories
with crafted service.yaml files, assert invalid config.service.name is rejected,
assert options.audit is refused, and verify a valid name/audit configuration
passes validation before failing later due to a missing runtime. Keep these
tests independent of mke2fs, kernel, rootfs, or hypervisor dependencies.
- Around line 159-186: Move the kernel and rootfs validation flow, including
validate_kernel_image and the associated existence/format checks, before the
create_ext4_image calls for the service and runtime disks in the execution
setup. Preserve all existing validation behavior and image-building behavior for
correctly provisioned hosts, but ensure invalid kernel or rootfs paths fail
before either image is generated.
In `@ignite-core/src/platform/apple_vz.rs`:
- Around line 53-62: Add an ignite-core regression test for AppleVzOrchestrator
that invokes boot and wait_and_teardown, asserting each returns
IgniteError::Runtime with the stable unsupported-operation message. Use the
existing test conventions and required callback/setup values, and verify both
methods independently.
In `@ignite-core/src/platform/firecracker.rs`:
- Around line 355-363: Extract the frame-size comparison from the VSOCK handling
path into a small helper such as frame_length_is_allowed(length: usize) -> bool,
and use it in the existing MAX_FRAME_BYTES validation. Add direct tests covering
both boundary cases: MAX_FRAME_BYTES is allowed, while MAX_FRAME_BYTES + 1 is
rejected.
In `@ignite-core/src/setup.rs`:
- Around line 49-93: Add unit tests covering validate_kernel_image and
validate_agent_is_static: verify valid ELF data without PT_INTERP passes, ELF
data containing PT_INTERP is rejected as dynamically linked, and
gzip-magic/non-ELF kernel input is rejected. Build temporary fixture files or
in-memory test inputs compatible with read_head, and assert both success and the
expected error results.
In `@ignite-guest-agent/src/main.rs`:
- Around line 136-165: Harden the mount flags at each call site: update the
/devtmpfs mount to include MS_NOSUID | MS_NOEXEC | MS_NODEV, add MS_NOSUID |
MS_NOEXEC | MS_NODEV to the proc and sysfs mounts, add MS_NOSUID | MS_NODEV
while preserving existing flags for the /app mount, and add MS_NOSUID while
preserving MS_RDONLY for /runtime. Keep the existing fatal and non-fatal error
handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f306948c-067a-4f10-8aee-332815d3ddde
📒 Files selected for processing (18)
.github/workflows/ci.ymlAGENTS.mdREADME.mddocs/api.mddocs/architecture.mddocs/threat-model.mdignite-cli/src/main.rsignite-core/src/execution.rsignite-core/src/orchestrator.rsignite-core/src/platform/apple_vz.rsignite-core/src/platform/firecracker.rsignite-core/src/setup.rsignite-guest-agent/src/main.rsignite-http/src/main.rsignite-http/src/server.rsignite-shared/src/lib.rsignite-shared/src/validation.rsinstall.sh
| - name: Verify guest agent links statically | ||
| run: | | ||
| cargo build --release --bin ignite-guest-agent \ | ||
| --target x86_64-unknown-linux-musl | ||
| AGENT=target/x86_64-unknown-linux-musl/release/ignite-guest-agent | ||
| file "$AGENT" | ||
| if readelf -l "$AGENT" | grep -qi 'interpreter'; then | ||
| echo "::error::Guest agent is dynamically linked. The guest rootfs has no dynamic loader, so it would fail to exec as init." | ||
| exit 1 | ||
| fi | ||
| echo "Guest agent is statically linked." |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the static-link check fail when readelf fails.
The step pipes readelf -l into grep. The exit status of a pipeline is the status of the last command, so a readelf failure (missing tool, unreadable file) is masked and the check reports success. Enable pipefail and inspect captured output instead.
Also consider matching the segment tag INTERP rather than the descriptive text, because the wording of readelf output can change between binutils versions.
🛡️ Proposed hardening
- name: Verify guest agent links statically
run: |
+ set -euo pipefail
cargo build --release --bin ignite-guest-agent \
--target x86_64-unknown-linux-musl
AGENT=target/x86_64-unknown-linux-musl/release/ignite-guest-agent
file "$AGENT"
- if readelf -l "$AGENT" | grep -qi 'interpreter'; then
+ HEADERS=$(readelf -l "$AGENT")
+ if grep -qE 'INTERP|interpreter' <<<"$HEADERS"; then
echo "::error::Guest agent is dynamically linked. The guest rootfs has no dynamic loader, so it would fail to exec as init."
exit 1
fi
echo "Guest agent is statically linked."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Verify guest agent links statically | |
| run: | | |
| cargo build --release --bin ignite-guest-agent \ | |
| --target x86_64-unknown-linux-musl | |
| AGENT=target/x86_64-unknown-linux-musl/release/ignite-guest-agent | |
| file "$AGENT" | |
| if readelf -l "$AGENT" | grep -qi 'interpreter'; then | |
| echo "::error::Guest agent is dynamically linked. The guest rootfs has no dynamic loader, so it would fail to exec as init." | |
| exit 1 | |
| fi | |
| echo "Guest agent is statically linked." | |
| - name: Verify guest agent links statically | |
| run: | | |
| set -euo pipefail | |
| cargo build --release --bin ignite-guest-agent \ | |
| --target x86_64-unknown-linux-musl | |
| AGENT=target/x86_64-unknown-linux-musl/release/ignite-guest-agent | |
| file "$AGENT" | |
| HEADERS=$(readelf -l "$AGENT") | |
| if grep -qE 'INTERP|interpreter' <<<"$HEADERS"; then | |
| echo "::error::Guest agent is dynamically linked. The guest rootfs has no dynamic loader, so it would fail to exec as init." | |
| exit 1 | |
| fi | |
| echo "Guest agent is statically linked." |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 15-59: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 49 - 59, Update the “Verify guest
agent links statically” workflow step to enable pipeline failure propagation and
capture the `readelf -l "$AGENT"` output before checking it. Make the step fail
when `readelf` cannot inspect the binary, and detect dynamic linking by matching
the stable `INTERP` segment tag rather than descriptive wording.
| | Limit privilege escalation | No shell (`/bin/sh`), compiler, or system utilities exist in the guest rootfs. | | ||
| | Bound runaway processes | Memory/vCPU limits are applied to Firecracker machine config, and a host watchdog force-terminates the VM when `timeoutMs` is exceeded. | | ||
| | Bound runaway processes | Memory/vCPU limits are applied to Firecracker machine config, and a host watchdog force-terminates the VM when `timeoutMs` is exceeded. The deadline is checked on every read, so a guest that writes output continuously cannot hold the watchdog open. | | ||
| | Bound host memory from guest output | VSOCK frame lengths are guest-controlled, so frames are capped (4 MiB each) and retained stdout/stderr is capped (8 MiB per stream) rather than allocated on the guest's word. | |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
State the allocation guarantee precisely.
“Allocated on the guest's word” is not a defined host-side check. It can imply that the host trusts the guest. State that the host validates each guest-declared frame length against the 4 MiB limit before allocation and separately caps retained stdout/stderr at 8 MiB per stream.
Proposed wording
-| Bound host memory from guest output | VSOCK frame lengths are guest-controlled, so frames are capped (4 MiB each) and retained stdout/stderr is capped (8 MiB per stream) rather than allocated on the guest's word. |
+| Bound host memory from guest output | VSOCK frame lengths are guest-controlled. The host validates each guest-declared frame length before allocation, caps frames at 4 MiB each, and caps retained stdout/stderr at 8 MiB per stream. |Based on the trust-boundary text in docs/threat-model.md, VSOCK metadata is attacker-controlled and should be described as guest-declared data.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Bound host memory from guest output | VSOCK frame lengths are guest-controlled, so frames are capped (4 MiB each) and retained stdout/stderr is capped (8 MiB per stream) rather than allocated on the guest's word. | | |
| | Bound host memory from guest output | VSOCK frame lengths are guest-controlled. The host validates each guest-declared frame length before allocation, caps frames at 4 MiB each, and caps retained stdout/stderr at 8 MiB per stream. | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/threat-model.md` at line 15, Update the “Bound host memory from guest
output” entry in the threat model to state that the host validates each
guest-declared VSOCK frame length against the 4 MiB limit before allocation, and
separately caps retained stdout and stderr at 8 MiB per stream. Remove the
undefined “allocated on the guest's word” phrasing.
| fn cleanup_vm_processes(&mut self) { | ||
| if let Some(mut child) = self.child_process.take() { | ||
| let _ = child.kill(); | ||
| let _ = child.wait(); | ||
| } | ||
| let _ = fs::remove_file(&self.api_socket_path); | ||
| self.vsock_listener = None; | ||
| if let Some(ref config) = self.config { | ||
| let vsock_listener_path = format!( | ||
| "{}{}", | ||
| config.vsock_uds_path.to_string_lossy(), | ||
| VSOCK_PORT_SUFFIX | ||
| ); | ||
| let _ = fs::remove_file(&config.vsock_uds_path); | ||
| let _ = fs::remove_file(&vsock_listener_path); | ||
| } | ||
| if let Some(ref path) = self.vsock_listener_path { | ||
| let _ = fs::remove_file(path); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Do not unlink the shared default API socket path.
cleanup_vm_processes always removes self.api_socket_path, and Drop calls it. If an orchestrator is created and dropped without configure, the path is still DEFAULT_API_SOCKET (/tmp/firecracker-api.sock), which is a fixed shared path. The drop then unlinks a socket file this instance never created, and it can disrupt another process.
Remove the file only when this instance owns a run-specific path.
🐛 Proposed guard
fn cleanup_vm_processes(&mut self) {
if let Some(mut child) = self.child_process.take() {
let _ = child.kill();
let _ = child.wait();
}
- let _ = fs::remove_file(&self.api_socket_path);
+ // Only unlink a run-specific socket; the default path is shared.
+ if self.api_socket_path != Path::new(DEFAULT_API_SOCKET) {
+ let _ = fs::remove_file(&self.api_socket_path);
+ }
self.vsock_listener = None;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn cleanup_vm_processes(&mut self) { | |
| if let Some(mut child) = self.child_process.take() { | |
| let _ = child.kill(); | |
| let _ = child.wait(); | |
| } | |
| let _ = fs::remove_file(&self.api_socket_path); | |
| self.vsock_listener = None; | |
| if let Some(ref config) = self.config { | |
| let vsock_listener_path = format!( | |
| "{}{}", | |
| config.vsock_uds_path.to_string_lossy(), | |
| VSOCK_PORT_SUFFIX | |
| ); | |
| let _ = fs::remove_file(&config.vsock_uds_path); | |
| let _ = fs::remove_file(&vsock_listener_path); | |
| } | |
| if let Some(ref path) = self.vsock_listener_path { | |
| let _ = fs::remove_file(path); | |
| } | |
| } | |
| fn cleanup_vm_processes(&mut self) { | |
| if let Some(mut child) = self.child_process.take() { | |
| let _ = child.kill(); | |
| let _ = child.wait(); | |
| } | |
| // Only unlink a run-specific socket; the default path is shared. | |
| if self.api_socket_path != Path::new(DEFAULT_API_SOCKET) { | |
| let _ = fs::remove_file(&self.api_socket_path); | |
| } | |
| self.vsock_listener = None; | |
| if let Some(ref config) = self.config { | |
| let _ = fs::remove_file(&config.vsock_uds_path); | |
| } | |
| if let Some(ref path) = self.vsock_listener_path { | |
| let _ = fs::remove_file(path); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ignite-core/src/platform/firecracker.rs` around lines 511 - 524, Update
cleanup_vm_processes so it removes self.api_socket_path only when the instance
has been configured with an owned run-specific API socket path; preserve cleanup
of the child process and other instance-owned socket paths. Ensure dropping an
unconfigured orchestrator never unlinks the shared DEFAULT_API_SOCKET.
| fn elf_needs_dynamic_loader(bytes: &[u8]) -> bool { | ||
| if !is_elf(bytes) || bytes.len() < 64 || bytes[4] != 2 || bytes[5] != 1 { | ||
| return false; | ||
| } | ||
| let phoff = u64::from_le_bytes(bytes[32..40].try_into().unwrap()) as usize; | ||
| let phentsize = u16::from_le_bytes(bytes[54..56].try_into().unwrap()) as usize; | ||
| let phnum = u16::from_le_bytes(bytes[56..58].try_into().unwrap()) as usize; | ||
| if phentsize < 4 { | ||
| return false; | ||
| } | ||
| (0..phnum).any(|i| { | ||
| let off = phoff + i * phentsize; | ||
| off + 4 <= bytes.len() | ||
| && u32::from_le_bytes(bytes[off..off + 4].try_into().unwrap()) == PT_INTERP | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use checked arithmetic in elf_needs_dynamic_loader and note the fail-open paths.
phoff, phentsize, and phnum come from the file bytes. phoff + i * phentsize and off + 4 can overflow usize. An overflow panics in debug builds and wraps in release builds, which can make the scan inspect the wrong offsets. Use checked_mul/checked_add and skip the entry when the arithmetic fails.
The function also returns false for 32-bit ELF, big-endian ELF, and for any binary whose program headers sit past ELF_HEADER_SCAN_BYTES. In those cases a dynamically linked agent passes validation and the guest fails to exec init. Consider returning a tri-state (static / dynamic / unknown) and rejecting unknown in validate_agent_is_static.
🛡️ Proposed overflow-safe iteration
(0..phnum).any(|i| {
- let off = phoff + i * phentsize;
- off + 4 <= bytes.len()
- && u32::from_le_bytes(bytes[off..off + 4].try_into().unwrap()) == PT_INTERP
+ let Some(off) = i.checked_mul(phentsize).and_then(|d| phoff.checked_add(d)) else {
+ return false;
+ };
+ let Some(end) = off.checked_add(4) else {
+ return false;
+ };
+ end <= bytes.len()
+ && u32::from_le_bytes(bytes[off..end].try_into().unwrap()) == PT_INTERP
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn elf_needs_dynamic_loader(bytes: &[u8]) -> bool { | |
| if !is_elf(bytes) || bytes.len() < 64 || bytes[4] != 2 || bytes[5] != 1 { | |
| return false; | |
| } | |
| let phoff = u64::from_le_bytes(bytes[32..40].try_into().unwrap()) as usize; | |
| let phentsize = u16::from_le_bytes(bytes[54..56].try_into().unwrap()) as usize; | |
| let phnum = u16::from_le_bytes(bytes[56..58].try_into().unwrap()) as usize; | |
| if phentsize < 4 { | |
| return false; | |
| } | |
| (0..phnum).any(|i| { | |
| let off = phoff + i * phentsize; | |
| off + 4 <= bytes.len() | |
| && u32::from_le_bytes(bytes[off..off + 4].try_into().unwrap()) == PT_INTERP | |
| }) | |
| } | |
| fn elf_needs_dynamic_loader(bytes: &[u8]) -> bool { | |
| if !is_elf(bytes) || bytes.len() < 64 || bytes[4] != 2 || bytes[5] != 1 { | |
| return false; | |
| } | |
| let phoff = u64::from_le_bytes(bytes[32..40].try_into().unwrap()) as usize; | |
| let phentsize = u16::from_le_bytes(bytes[54..56].try_into().unwrap()) as usize; | |
| let phnum = u16::from_le_bytes(bytes[56..58].try_into().unwrap()) as usize; | |
| if phentsize < 4 { | |
| return false; | |
| } | |
| (0..phnum).any(|i| { | |
| let Some(off) = i.checked_mul(phentsize).and_then(|d| phoff.checked_add(d)) else { | |
| return false; | |
| }; | |
| let Some(end) = off.checked_add(4) else { | |
| return false; | |
| }; | |
| end <= bytes.len() | |
| && u32::from_le_bytes(bytes[off..end].try_into().unwrap()) == PT_INTERP | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ignite-core/src/setup.rs` around lines 29 - 44, Update
elf_needs_dynamic_loader to use checked_mul and checked_add when calculating
each program-header offset and the end of the PT_INTERP field, skipping entries
whose arithmetic overflows instead of panicking or wrapping. Also distinguish
static, dynamic, and unknown ELF classifications so unsupported formats and
headers outside ELF_HEADER_SCAN_BYTES become unknown, then update
validate_agent_is_static to reject unknown results.
| let api_key = std::env::var("IGNITE_API_KEY").ok(); | ||
| if api_key.is_none() { | ||
| tracing::warn!( | ||
| "IGNITE_API_KEY is not set: this server will execute services for any caller that \ | ||
| can reach it. Set IGNITE_API_KEY, or bind to localhost only." | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject blank API-key configuration.
IGNITE_API_KEY="" becomes Some(""). RequireAuth then accepts Authorization: Bearer , and startup does not emit the unauthenticated-server warning.
ignite-http/src/main.rs#L26-L32: Return a configuration error whenIGNITE_API_KEYis empty or whitespace-only.ignite-cli/src/main.rs#L1031-L1037: Apply the same validation through shared configuration parsing.docs/api.md#L73-L76: State that a configured API key must be nonempty.
As per coding guidelines, “Do not advertise security controls that are not enforced; fail loudly when a requested flag cannot be honored, as --audit does.”
📍 Affects 3 files
ignite-http/src/main.rs#L26-L32(this comment)ignite-cli/src/main.rs#L1031-L1037docs/api.md#L73-L76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ignite-http/src/main.rs` around lines 26 - 32, Reject empty or
whitespace-only IGNITE_API_KEY values during shared configuration parsing, while
preserving the warning only for an unset key; update ignite-http/src/main.rs
lines 26-32 and ignite-cli/src/main.rs lines 1031-1037 so both startup paths
return a configuration error instead of accepting blank credentials, and update
docs/api.md lines 73-76 to state that configured API keys must be nonempty.
Source: Coding guidelines
| // Drop buckets that have fully aged out so the map cannot grow without | ||
| // bound across many distinct clients. | ||
| let window = self.window; | ||
| map.retain(|_, timestamps| { | ||
| timestamps.retain(|&t| now.duration_since(t) < window); | ||
| !timestamps.is_empty() | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid a full rate-limit scan for every request.
This code scans every bucket and every retained timestamp while it holds the only mutex. With the configured 10,000-bucket cap and 60 requests per bucket, one request can inspect about 600,000 timestamps.
Use per-client pruning plus an expiry index for idle buckets. Do not scan the full map on the request path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ignite-http/src/server.rs` around lines 57 - 63, Replace the full-map retain
block in the rate-limit request path with per-client timestamp pruning and an
expiry index for idle buckets. Prune only the current client’s timestamps while
holding the mutex, and process expired buckets through the index rather than
scanning all entries; preserve the existing window-based expiration and
bucket-cap behavior.
| let origins: Vec<axum::http::HeaderValue> = state | ||
| .allowed_origins | ||
| .iter() | ||
| .filter_map(|o| o.parse().ok()) | ||
| .collect(); | ||
| router.layer( | ||
| CorsLayer::new() | ||
| .allow_origin(tower_http::cors::Any) | ||
| .allow_origin(AllowOrigin::list(origins)) | ||
| .allow_methods([axum::http::Method::GET, axum::http::Method::POST]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the resolved tower-http dependency and all CORS configuration paths.
fd -a '^Cargo\.(toml|lock)$' . -x rg -n -C2 'tower-http' {}
rg -n -C3 'IGNITE_CORS_ORIGINS|AllowOrigin::list|allow_origin' ignite-http/src ignite-cli/srcRepository: dev-dami/ignite
Length of output: 695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the server.rs file and relevant CORS configuration definitions/usages.
fd -a '^server\.rs$' . | while read -r f; do
echo "===== $f ====="
wc -l "$f"
done
echo "===== server.rs CORS section ====="
sed -n '230,305p' ignite-http/src/server.rs
echo "===== CORS config references ====="
rg -n -C4 'IG(NITE|NITE_CORS)?_CORS|cors|origin|Origins|ServiceState|allowed_origins' ignite-http/src ignite-cli/src docs README.md Cargo.toml Cargo.lock 2>/dev/null || trueRepository: dev-dami/ignite
Length of output: 9248
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== ignite-http/src/main.rs CORS parsing section ====="
sed -n '1,70p' ignite-http/src/main.rs
echo "===== ignite-cli/src/main.rs CORS parsing section ====="
sed -n '1020,1070p' ignite-cli/src/main.rs
echo "===== Cargo.lock tower-http package ====="
sed -n '1230,1250p' Cargo.lockRepository: dev-dami/ignite
Length of output: 5332
🌐 Web query:
tower-http 0.5.2 AllowOrigin list panics input contains asterisk documentation
💡 Result:
In tower-http 0.5.2, the AllowOrigin::list method explicitly panics if the provided iterator of origins contains the wildcard character () [1][2]. This behavior was implemented to enforce correct usage of the CORS middleware, as the wildcard is intended to be handled via the AllowOrigin::any method rather than being included in a list of specific origins [1][2]. If you need to allow all origins, you should use AllowOrigin::any instead of passing a wildcard to AllowOrigin::list [1][2]. The documentation for AllowOrigin::list notes this requirement under its Panics section, stating: Wildcard origin () cannot be passed to AllowOrigin::list. Use AllowOrigin::any instead [2].
Citations:
- 1: https://docs.rs/tower-http/latest/tower_http/cors/struct.AllowOrigin.html
- 2: https://github.com/tower-rs/tower-http/blob/main/tower-http/src/cors/allow_origin.rs
Reject invalid CORS origins before router construction.
IGNITE_CORS_ORIGINS currently allows malformed values and * to reach AllowOrigin::list; the latter panics when CORS is enabled. Parse origins once where IGNITE_CORS_ORIGINS is read, reject invalid or wildcard values, fail when no valid origins remain, and return a configuration error instead of silently disabling CORS or crashing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ignite-http/src/server.rs` around lines 271 - 279, Update the
IGNITE_CORS_ORIGINS configuration parsing before router construction to parse
each origin once, reject malformed and wildcard values, and return a
configuration error for any invalid entry or when no valid origins remain. Store
and reuse the validated HeaderValue list in the CORS setup around
AllowOrigin::list, removing the current filter_map behavior that silently drops
invalid origins.
Source: Coding guidelines
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn rate_limiter_allows_up_to_the_cap_then_blocks() { | ||
| let limiter = RateLimiter::new(3, 60); | ||
| for i in 0..3 { | ||
| assert!(limiter.check("1.2.3.4".into()), "request {i} should pass"); | ||
| } | ||
| assert!( | ||
| !limiter.check("1.2.3.4".into()), | ||
| "4th request must be denied" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn rate_limiter_buckets_are_per_client() { | ||
| let limiter = RateLimiter::new(2, 60); | ||
| assert!(limiter.check("1.1.1.1".into())); | ||
| assert!(limiter.check("1.1.1.1".into())); | ||
| assert!(!limiter.check("1.1.1.1".into())); | ||
| // A different client must not inherit the exhausted bucket. | ||
| assert!(limiter.check("2.2.2.2".into())); | ||
| } | ||
|
|
||
| #[test] | ||
| fn rate_limiter_releases_bucket_after_window() { | ||
| // A zero-length window means every prior timestamp is already expired, | ||
| // so entries must be reclaimed rather than accumulating forever. | ||
| let limiter = RateLimiter::new(1, 0); | ||
| assert!(limiter.check("9.9.9.9".into())); | ||
| assert!(limiter.check("9.9.9.9".into())); | ||
| assert_eq!( | ||
| limiter.requests.lock().unwrap().len(), | ||
| 1, | ||
| "expired buckets should not accumulate" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn rate_limiter_bucket_count_is_bounded() { | ||
| let limiter = RateLimiter::new(5, 60); | ||
| for i in 0..(MAX_RATE_LIMIT_BUCKETS + 500) { | ||
| limiter.check(format!("10.0.{}.{}", i / 256, i % 256)); | ||
| } | ||
| assert!( | ||
| limiter.requests.lock().unwrap().len() <= MAX_RATE_LIMIT_BUCKETS, | ||
| "bucket map exceeded its bound" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn health_reports_the_real_crate_version() { | ||
| // Previously hardcoded to 0.1.0 while the crate was 0.9.0. | ||
| assert_eq!(env!("CARGO_PKG_VERSION"), "0.9.0"); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add router tests for authentication and CORS decisions.
These tests cover rate-limit counts only. Add tests that accept a valid API key and configured origin. Add tests that reject missing or invalid keys and unlisted origins.
As per coding guidelines, “When security logic changes, include tests proving both allowed and blocked behavior.” Based on learnings, “When behavior changes, add or update tests in the relevant crate.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ignite-http/src/server.rs` around lines 290 - 346, Add router-level tests
alongside the existing tests that verify authentication and CORS decisions:
accept requests with a valid API key and configured origin, and reject requests
with missing or invalid keys and origins not present in the allowed
configuration. Reuse the router construction and security configuration symbols
already defined in the module, covering both allowed and blocked outcomes
without changing the rate-limiter tests.
Sources: Coding guidelines, Learnings
| EOF | ||
| echo -e "${NC}" | ||
| echo -e " ${DIM}Run JS/TS microservices in Docker${NC}" | ||
| echo -e " ${DIM}Run JS/TS microservices in secure microVMs${NC}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Avoid the blanket “secure” claim in the installer banner.
The current implementation documents several security controls that are not implemented. Describe the enforced mechanism instead.
Proposed wording
- echo -e " ${DIM}Run JS/TS microservices in secure microVMs${NC}"
+ echo -e " ${DIM}Run JS/TS microservices in Firecracker microVMs${NC}"As per coding guidelines, do not advertise a security control that is not enforced.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo -e " ${DIM}Run JS/TS microservices in secure microVMs${NC}" | |
| echo -e " ${DIM}Run JS/TS microservices in Firecracker microVMs${NC}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install.sh` at line 26, Update the installer banner’s echo text to describe
the actual enforced microVM mechanism without using the blanket “secure” claim;
leave the surrounding banner formatting and behavior unchanged.
Source: Coding guidelines
|
|
||
| - **Dual-Hypervisor Core**: Uses KVM-backed Firecracker on Linux, and native `Virtualization.framework` on macOS. | ||
| - **KVM-backed Firecracker**: Each service runs in its own microVM with a separate guest kernel. (A macOS `Virtualization.framework` backend is planned; see Status above.) | ||
| - **Host-Reliant Disk Mounts**: The guest microVM has no shell, utilities, or libraries. Service code and language runtimes (Bun, Node, Deno, QuickJS) are compiled on the host and attached as read-only virtual block devices (`/dev/vdb` and `/dev/vdc`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe runtime provisioning accurately.
The supplied setup flow builds the guest agent, while runtime binaries come from the runtime directory. The phrase “language runtimes ... are compiled on the host” implies a build step that this workflow does not perform. Use “installed or downloaded on the host, then packaged into read-only block devices.”
Proposed wording
- Service code and language runtimes (Bun, Node, Deno, QuickJS) are compiled on the host and attached as read-only virtual block devices (`/dev/vdb` and `/dev/vdc`).
+ Service code and installed runtime binaries (Bun, Node, Deno, QuickJS) are packaged on the host and attached as read-only virtual block devices (`/dev/vdb` and `/dev/vdc`).Based on the supplied ignite-core provisioning flow, only the guest agent is built; runtime binaries are read from runtime_src_path.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Host-Reliant Disk Mounts**: The guest microVM has no shell, utilities, or libraries. Service code and language runtimes (Bun, Node, Deno, QuickJS) are compiled on the host and attached as read-only virtual block devices (`/dev/vdb` and `/dev/vdc`). | |
| - **Host-Reliant Disk Mounts**: The guest microVM has no shell, utilities, or libraries. Service code and installed runtime binaries (Bun, Node, Deno, QuickJS) are packaged on the host and attached as read-only virtual block devices (`/dev/vdb` and `/dev/vdc`). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 34, Update the “Host-Reliant Disk Mounts” description in
README.md to state that runtime binaries are installed or downloaded on the host
and then packaged into read-only virtual block devices, while retaining that the
guest agent is built by the setup flow. Remove the claim that Bun, Node, Deno,
and QuickJS are compiled on the host.
1 participant
Summary
The guest boot chain had four independent blockers that each stopped execution before user code could run, plus several host-side security gaps. This fixes all of them and corrects docs that described a system ahead of the code.
Test coverage goes from 1 test to 29.
Boot chain
init=mismatch — boot args passedinit=/usr/local/bin/ignite-guest-agentwhilecreate_rootfsinstalled the agent at/sbin/init. Both now use a sharedGUEST_INIT_PATH.--target x86_64-unknown-linux-musl, producing a glibc-linked binary for a rootfs with no libc. It now targets musl, and bothbuild_guest_agentandcreate_rootfsreject an ELF carryingPT_INTERP.mkdiron a read-only rootfs. Mount points are baked into the image,devtmpfsis mounted first (without it/dev/vdbdoes not exist), and a failed disk mount is now fatal rather than logged and ignored.download_kernelcopied/boot/vmlinuz-*, a compressed bzImage Firecracker cannot boot. It now requires an uncompressed ELFvmlinuxand names any compressed images it found.The VSOCK listener also binds before
InstanceStart, so the guest cannot exhaust its connect retries before the host is listening.Security
service.namefromservice.yamlreached a host path thatboot()unlinks, so../..in that field could remove host files. It is now validated and sanitized, and the socket path is pinned under/tmp.WouldBlockbranch, so a guest writing output continuously ran pasttimeoutMsforever. It is now checked every iteration.X-Forwarded-For, never evicted buckets, and compared API keys non-constant-time. Now keys on the transport peer, bounds bucket count, compares in constant time.IGNITE_CORS_ORIGINS; both entry points warn whenIGNITE_API_KEYis unset.Flags that parsed but did nothing
--runtimeand--console-outare wired through.--auditnow fails loudly — silently accepting it reports a clean audit for code that was never audited.cpuLimitrounds up instead of truncating0.5to 0 vCPUs. Runtime versions resolve toruntimes/<name>@<version>with a logged fallback. Cold-start timing is reported only when the guest emits it rather than fabricated asmin(duration, 200).Also fixes
send_put_udstreating a 4xx whose body echoedHTTP/1.1 200as success,serve --host localhostsilently falling back to127.0.0.1instead of resolving, and the KVM check instatusmisreading mode bits so a healthy device reported "check user group".Docs
AGENTS.mddocumented a Bun/TypeScript monorepo (packages/*, "never use npm") that no longer exists — any agent following it would have been lost immediately. Rewritten for the Rust workspace. README/docs claims about macOS support, audit mode, and runtime provisioning are corrected, and the threat model gains a "Not Implemented" section covering audit mode, the Firecracker jailer, macOS, and unauthenticated-by-default HTTP.Verification
cargo fmt --check,clippy -D warnings, andcargo test --workspace(29 tests) all pass, including under CI'sRUSTFLAGS="-D warnings".The musl agent was confirmed
static-piewith noPT_INTERPand byte-identical to/sbin/initextracted from the built rootfs image at mode 755. The ELF detector was cross-checked against real static and dynamic binaries, and the static-linking guard confirmed to refuse a glibc agent. CI now asserts this invariant so it cannot regress silently.VM-level execution could not be verified. The dev host has no
firecrackerbinary and no uncompressedvmlinux, so the guest-side path — devtmpfs mount, VSOCK handshake, agent running as PID 1, watchdog behavior under a chatty guest — is fixed by inspection only. A real boot on a KVM host is the one thing worth doing before trusting this end to end.Not addressed
Firecracker still runs unjailed (no
jailer, no seccomp),ignite setupstill provisions Bun only, and preflight defaults still fail a 128 MB service around 50 dependencies. All three are now documented as known gaps rather than silently absent.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation